note: This assumption is broken by the network-manager-openconnect plugin, as described in LP#2080301. We might be able to special case the vpn-secrets group similarly as we did with tc, to keep things simple... Although, that might make us run into similar issues in the future for other "edge cases".

note: I think this might leak memory, iff the key or value already exists in *list, as we did not supply a key_destroy_func/value_destroy_func above when creating the hash-table (e.g. using g_hash_table_new_full)

In this particular case, groups is created by g_key_file_get_groups which will not add repetitions to the result.

Although, there is a different memory leak there: thanks to the continue in the next line it will not free the keys array.

question: If we walk through a mapping, what's the escaped_value why do we need it here? Is it defined at all?

oh, that's a good point. The escaped_value is only relevant when the value type is scalar. I'll move it to inside the if statement.

nitpick: _new_format is very generic. Maybe better call it test_passthrough_basic_mapping.

(Also in other places where "new_format" is used.)

question: should the result rather be key=b here. So that the value defined later wins?

also followup question: what happens if we mix and match the old format?

e.g.

passthrough:
  connection:
    name: "another name"
  connection.name: "some name"

I'd argue the value defined in the new (mapping) format should take precendence. WDYT?

hmm yeaaah it probably should be key=b in the end. But that's actually a problem in the handle_generic_map function. It will error out if it finds the same key with different values (see below). Arguably this logic should be used everywhere else then...

$ cat /tmp/fakeroot/etc/netplan/a.yaml
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    renderer: NetworkManager
    enp5s0:
      dhcp4: true
      openvswitch:
        other-config:
          one: 1
          one: 2
$ netplan  get --root-dir /tmp/fakeroot
Command failed: /tmp/fakeroot/etc/netplan/a.yaml:10:11: Error in network definition: duplicate map entry 'one'
          one: 1
          ^

For the second question, mixed format is supported. Although, if the key is set using the old style first, the parser will ignore it if it's found again using the new format. The problem is the same described above, but for these cases I'm ignoring the error.

I could create a new handle_generic_map only for the passthrough settings... but in this case I suppose the same logic should be used across all the parser and not only here...

And I just realized that netplan set will not work if you try to update these fields, again, due to the way handle_generic_map works.

Adding and nullifying fields work fine.

It also doesn't work for openvswitch:

# netplan get
network:
  version: 2
  renderer: NetworkManager
  ethernets:
    enp5s0:
      dhcp4: true
      openvswitch:
        other-config:
          one: "1"
root@oracular-deleteme:~# netplan set ethernets.enp5s0.openvswitch.other-config.one=2
Command failed: Error in network definition: duplicate map entry 'one'

nitpick: Maybe call it test_passthrough_wifi_mapping to be more precise.

I don't think we need the !nd->backend_settings.passthrough here any more. key is only defined if nd->backend_settings.passthrough was accessible in the previous if-statement.

question: Do we need to change the escaping in existing netplan get calls, out in the wild? That might have the potential to break the snapd integration used in Ubuntu Core.

Yes, that (the escaping with \) will not work anymore if you are using get and set with settings inside the passthrough section. We probably shouldn't backport this change to LTS releases (like, never), even if we don't give any guarantees about stability for passthrough block.